-
Notifications
You must be signed in to change notification settings - Fork 179
Update PPL Command Documentation #4562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: ritvibhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Hi @aalva500-prog, thanks for bringing that up! I have now updated the functions documentation as well |
|
Hi @ritvibhatt , reviewed |
Swiddis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crashes my web viewer of the diff, lgtm
| ============================================================== ================== ======================== ============================================================================================== | ||
| Command Name Version Introduced Current Status Command Description | ||
| ============================================================== ================== ======================== ============================================================================================== | ||
| `search command <cmd/search.rst>`_ 1.0 stable (since 1.0) Retrieve documents from the index. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on the summaries
| * **Functions** | ||
|
|
||
| - `top command <cmd/top.rst>`_ | ||
| - `Aggregation Functions <functions/aggregation.rst>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Should we have descriptions or some further info for these too?
Not clear how to find e.g. the SUM function (is it an aggregation function or a math function?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that makes sense to do! Was thinking of a follow up PR to update some of the functions content as well as add to index.rst since this PR is already pretty big
docs/user/ppl/cmd/stats.rst
Outdated
| | ["Amber","Dale","Hattie","Nanette"] | | ||
| +-------------------------------------+ | ||
| * aggregation: mandatory. An aggregation function. | ||
| * bucket_nullable: optional. Controls whether the stats command includes null buckets in group-by aggregations. When set to ``false``, the aggregation ignores records where the group-by field is null, resulting in faster performance by excluding null bucket. **Default:** determined by ``plugins.ppl.syntax.legacy.preferred``: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ritvibhatt can you review your preview pages. It seems introduce many unexpected formatting.
For example, lines of bucket_nullable and span-expression in your preview
https://github.com/ritvibhatt/sql/blob/update-docs/docs/user/ppl/cmd/stats.rst are bold.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @LantaoJin, just rechecked previews for all of the files and fixed all of the nested bullet points to be formatted properly thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ritvibhatt the issue I mentioned above is still existing: the whole line of bucket_nullable is bold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I was missing a commit the issue should be fixed now. Have double checked the preview
|
This PR is stalled because it has been open for 2 weeks with no activity. |
f43765d to
e9f085b
Compare
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: Ritvi Bhatt <[email protected]>
|
can u help to fix the typo in https://github.com/ritvibhatt/sql/blob/update-docs/docs/user/ppl/functions/condition.rst#regexp-match change to |
Signed-off-by: Ritvi Bhatt <[email protected]>
@LantaoJin Updated to be |
LantaoJin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will keep eyes on CI.
This PR standardizes the structure and content of PPL command documentation files to improve consistency and user experience.
Changes Made
Documentation Structure Standardization
Implemented consistent section ordering across all PPL command files:
Content Cleanup and Modernization
3.3.0") to keep documentation current and reduce maintenance overhead
functions file (
/functions/aggregations.rst) for better organizationRelated Issues
Resolves #[Issue number to be closed when this PR is merged]
#4220
Check List
--signoffor-s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.